Skip to content

Conversation

inglorion
Copy link
Contributor

Previously, we relied on _NEWLIB_VERSION being defined to detect if we are building with Newlib or its derivative picolibc. This is somewhat unreliable, because whether or not NEWLIB_VERSION is defined depends on if any headers from the C library have been included or not. This could lead to inconsistent definitions, for example as observed in bug #152763. The code contained a workaround that detected picolibc by looking for its header file, with a TODO to replace this logic with a different mechanism. This change addresses that TODO by handling Newlib and picolibc similarly to how we handle musl: by providing a LIBCXX_HASLIBC option and a corresponding LIBCPP_HAS macro to be used in code.

Specifically:

  • For Newlibc, we provide LIBCXX_HAS_NEWLIB_LIBC and _LIBCPP_HAS_NEWLIB_LIBC.

  • For picolibc, we provide LIBCXX_HAS_PICOLIBC and _LIBCPP_HAS_PICOLIBC.

Existing code has been rewritten so that instead of checking for defined(_NEWLIB_VERSION) it checks for
_LIBCPP_HAS_NEWLIB_LIBC || _LIBCPP_HAS_PICOLIBC.

Previously, we relied on _NEWLIB_VERSION being defined to detect if we
are building with Newlib or its derivative picolibc. This is somewhat
unreliable, because whether or not _NEWLIB_VERSION is defined depends
on if any headers from the C library have been included or not. This
could lead to inconsistent definitions, for example as observed in
bug llvm#152763. The code contained a workaround that detected picolibc by
looking for its header file, with a TODO to replace this logic with
a different mechanism. This change addresses that TODO by handling
Newlib and picolibc similarly to how we handle musl: by providing a
LIBCXX_HAS_*LIBC option and a corresponding _LIBCPP_HAS_* macro to be
used in code.

Specifically:

 - For Newlibc, we provide LIBCXX_HAS_NEWLIB_LIBC and
   _LIBCPP_HAS_NEWLIB_LIBC.

 - For picolibc, we provide LIBCXX_HAS_PICOLIBC and
   _LIBCPP_HAS_PICOLIBC.

Existing code has been rewritten so that instead of checking for
defined(_NEWLIB_VERSION) it checks for
_LIBCPP_HAS_NEWLIB_LIBC || _LIBCPP_HAS_PICOLIBC.
@inglorion
Copy link
Contributor Author

Some open questions:

  1. This now requires an option to be set. We can probably autodetect the correct value. Should we? How would we implement that?
  2. Instead of having boolean options for various libcs, should we have a single option to select the libc flavor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant